streamline layout and enhance community social proof#226
Conversation
There was a problem hiding this comment.
Thanks for putting this together. The visual design and the use of live data look fantastic!
I have a few architectural notes to help us polish this up before merging:
- Lockfiles (
website/bun.lock&website/yarn.lock) It looks like bun was used locally. This project strictly uses yarn, so committing bun.lock could cause confusion for other contributors and break our CI.
- Suggestion: Please delete
bun.lock, runyarn installto cleanly restoreyarn.lock, and update the PR.
I have highlighted some Rate limit issues and some nick picks before. other than that I have a few nit picks on the UI design.
- For consistency and visual pull poposes purposes, kindly restore the rounded joint intersect to the original orange rounded corner and center it since you have removed the other
Visit GitHub Repositoryfrom this location.
The goal is for this to be the primary button visitors want to click on, so it should be a different colot i.e: Orange for Prominence.
- As the Scondary button, again, we can maintain the rounded corners but consider having this button lead to the issues page like your initial PR. Seeing as it is a secondary button, consider having it white. This should direct the visitor's eyes after the join intersect button.
|
|
||
| useEffect(() => { | ||
| const repo = 'IntersectMBO/developer-experience'; | ||
| fetch(`https://api.github.com/repos/${repo}/contributors`) |
There was a problem hiding this comment.
Currently this fetches data directly from the GitHub API on the client side. Unauthenticated requests to /contributors are limited to 60 requests/hour, and the Search API is even stricter at 10 requests/minute. In a production environment, just a few concurrent visitors will quickly exhaust this limit and break the stats.
Link: GitHub Docs: Primary rate limit for unauthenticated users
| async function fetchPulse() { | ||
| try { | ||
| // Fetching from GitHub Search API to get aggregate counts | ||
| const [prsRes, openIssuesRes, closedIssuesRes] = await Promise.all([ |
There was a problem hiding this comment.
Just like the previuous comment. This also fetches directly from GitHub. Kindly look into the rate limit issue.
For both cases consider fetching this data at build time instead. Write a prebuild script to save the data to a local JSON file, then have the components import that JSON. Let me know if you need help setting this up!
| import styles from './styles.module.css'; | ||
|
|
||
| export default function Contributors() { | ||
| const [contributors, setContributors] = useState<any[]>([]); |
There was a problem hiding this comment.
The any type is used for contributor data.
Consider defining a minimal interface (e.g., interface Contributor { login: string; avatar_url: string; html_url: string; }) to keep types strict.
| .then(response => response.json()) | ||
| .then(data => { | ||
| if (Array.isArray(data)) { | ||
| const validContributors = data.filter((user: any) => |
There was a problem hiding this comment.
Same here. Let's refrain using any in favour of propper typed types.
| return ( | ||
| <div className={styles.contributorsContainer}> | ||
| <div className={styles.contributorsList}> | ||
| {contributors.map((user: any) => ( |
There was a problem hiding this comment.
Refer to previuous comments on the use any.
| .catch(error => console.error('Error fetching contributors:', error)); | ||
| }, []); | ||
|
|
||
| if (contributors.length === 0) { |
There was a problem hiding this comment.
Loading states are handled, but network failures are not.
Consider adding a fallback error state so the UI doesn't silently break or hang if a fetch fails
| openIssues: openIssues.total_count !== undefined ? openIssues.total_count : '-', | ||
| closedIssues: closedIssues.total_count !== undefined ? closedIssues.total_count : '-' | ||
| }); | ||
| } catch (error) { |
There was a problem hiding this comment.
Loading states are handled, but network failures are not.
Consider adding a fallback error state so the UI doesn't silently break or hang if a fetch fails.
2 participants
The purpose of this PR is to make the Community Section more engaging.
Related to #218Type of Change
Check the type of change this PR introduces:
Changes Made
Contributors.tsxcomponent that dynamically fetches and displays real-time contributor data from the GitHub API.Checklist
Ensure you have completed the following steps before submitting your PR:
mainormasterbranch.